Added an 'npr_image_crop_url' hook. #40
Conversation
|
@mike-douglas Sorry about the merge conflicts. It appears by adding in my suggestions to PR#39 caused the conflict. But it should be very quick to review, if you are willing. |
…ying to use them!
There was a problem hiding this comment.
Hi Mike! Just added a few comments, which are related to resolving the conflicts between the changes made from #39 that are now in master. If you could refactor this work based on that I would be happy to merge!
Thanks!
| @@ -170,6 +176,15 @@ function nprstory_api_query_callback( $i ) { | |||
|
|
|||
| } | |||
|
|
|||
| function ds_npr_api_query_tags_callback( $i ) { | |||
There was a problem hiding this comment.
Could you remove this from your patch to prevent the conflict from #39?
| @@ -72,6 +74,10 @@ function nprstory_settings_init() { | |||
| //ds_npr_query_publish_ | |||
| add_settings_field( 'ds_npr_query_publish_' . $i, 'Publish Stories ' . $i, 'nprstory_api_query_publish_callback', 'ds_npr_api_get_multi_settings', 'ds_npr_api_get_multi_settings', $i ); | |||
| register_setting( 'ds_npr_api_get_multi_settings', 'ds_npr_query_publish_' . $i , 'nprstory_validation_callback_select'); | |||
|
|
|||
There was a problem hiding this comment.
Could you remove this from your patch to prevent the conflict from #39?
| @@ -56,6 +56,8 @@ function nprstory_settings_init() { | |||
| add_settings_section( 'ds_npr_api_get_multi_settings', 'NPR API multiple get settings', 'nprstory_api_get_multi_settings_callback', 'ds_npr_api_get_multi_settings' ); | |||
|
|
|||
| add_settings_field( 'ds_npr_num', 'Number of things to get', 'nprstory_api_num_multi_callback', 'ds_npr_api_get_multi_settings', 'ds_npr_api_get_multi_settings' ); | |||
|
|
|||
| add_settings_field( 'ds_npr_num', 'Number of things to get', 'nprstory_api_num_multi_callback', 'ds_npr_api_get_multi_settings', 'ds_npr_api_get_multi_settings' ); | |||
There was a problem hiding this comment.
Could you remove this from your patch to prevent the conflict from #39?
| @@ -54,7 +54,7 @@ public static function nprstory_cron_pull() { | |||
| if ( $pub_option == 'Publish' ) { | |||
| $pub_flag = TRUE; | |||
| } | |||
| $story = $api->update_posts_from_stories($pub_flag); | |||
| $story = $api->update_posts_from_stories($pub_flag, "query_number={$i}" ); | |||
There was a problem hiding this comment.
The second argument for update_posts_from_stories just takes an integer
| * @return int|null $post_id or null | ||
| */ | ||
| function update_posts_from_stories( $publish = TRUE ) { |
There was a problem hiding this comment.
If you could, resubmit your PR with the most recent master branch, which has the $qnum parameter added to this function instead of an $opts array or WP arg string
There was a problem hiding this comment.
I will be happy to remove my last changeset if that's what you would prefer.
But before I do may I ask to make sure you were aware they were based on the suggestions I made to PR #39? I added comments inline to the changeset but I don't see the comments I added: https://github.com/nprds/nprapi-wordpress/pull/39/files
I basically suggested that you future proof and not add a numeric 2nd parameter to update_posts_from_stories() but instead add an $opts array that would contain the query_number so that in future if you need to add other parameters you would have end up with a list of many ordinal parameters.
But of course, feel free to decline this modification if you feel it is unimportant in which case I will revert that last commit from my PR.
We are running into a problem where an extra large image (
height="4727"andwidth="6306") is causing the Nginx server to just bail with an502 Bad GatewayinWP_Image_Editor_Imagick->thumbnail_image()on the$this->image->resizeImage( $dst_w, $dst_h, $filter, 1 )line when$dst_wand$dst_hare large. Not always, but sometimes.So I've added an
'npr_image_crop_url'hook to allow a site to catch extra large images and perform alternate processing on them (such as having them resized using rsz.io, like this: 1200px wide image.I hope you can review this pull request soon and also that you will accept it, to allow Atlanta's WABE.org to continue using the "official" plugin.